Skip to content

Glasgow | ITP May 2025 | Adiyah Farhan | Sprint 3 | Quote Generator App #742

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

AdiyahFarhan
Copy link

Learners, PR Template

Self checklist

  • I have committed my files one by one, on purpose, and for a reason
  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • I have tested my changes
  • My changes follow the style guide
  • My changes meet the requirements of this task

Changelist

Implement the "Quote Generator App" so that It can display one random quote from the array of quotes on page loads and when the user clicks "New Quote" button, a new random quote will be generated.

Questions

No

@AdiyahFarhan AdiyahFarhan added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Aug 2, 2025
@@ -3,13 +3,16 @@
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>Title here</title>
<script defer src="quotes.js"></script>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain why the originally script tag was up on the head tag (not in the bottom of body) and what defer attribute does here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we use <script> tag up on the tag, without the defer attribute then it downloads and executes the script before parsing the complete file. As a result, the script that tries to access the elements in the that haven't been parsed yet, t will return null because the element doesn’t exist in the DOM yet.

But if we use a defer attribute, it will just download the script on the head but executes the script after the is fully parsed. So by adding defer attribute, we can avoid any blocking issues that makes script to behave improperly.

@day-lee
Copy link

day-lee commented Aug 5, 2025

Good job! It works as intended and code is well structured. I left a comment. Please have a look!

@day-lee day-lee added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Aug 5, 2025
…sn't make any affect in functionality of the app
@AdiyahFarhan
Copy link
Author

Hi @day-lee ,
Please refer my comment for the explanation of using script tag in head tag of html with defer attribute. Also I have restored this in the index.html, the functionality was the same as before.

Thanks for your valuable feedback and draw my attention to towards it.

@AdiyahFarhan AdiyahFarhan added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Aug 9, 2025
@day-lee day-lee added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Aug 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Complete Volunteer to add when work is complete and all review comments have been addressed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants